@claude

Claude finished @MaryWylde's task in 2m 37s —— View job

PR Review: Feat/library

• Gather PR context
• Review API layer (src/api/library/)
• Review component architecture (atoms, molecules, contexts)
• Review config/infrastructure changes (next.config.js, package.json)
• Review modifications to existing components (Header, Navbar, UserProfile)
• Post comprehensive review feedback

Good foundational work — the atomic component hierarchy, Zod+RHF form validation, FLIP animation on card reorder, and the optimistic-with-rollback pattern in AddObjectModal are all well-done. Below are the issues worth resolving before merge.

Bugs

1. AuthContext — logout doesn't reset in-memory state

handleLogout clears the cookie but never calls setToken(null) or setAccountData(null). Any component reading token or accountData from context will continue to see the logged-in user until a hard refresh. Fix this →

2. GlobalStateContext — double fetch of libraries on first authenticated load

Two useEffects both call refetchLibraries() on mount when a token exists. The unconditional first effect ([refetchLibraries]) fires and doesn't set didAttemptLibrariesLoad.current, so the second effect's guard doesn't fire, and it calls refetchLibraries() a second time. Fix this →

3. AddObjectModal — "Select tag to reorder" dropdown is dead UI

The tag-filter dropdown in step 2 renders with no onChange or value, so it neither reads nor writes any state. It does nothing.

// src/components/library/organisms/AddObjectModal/AddObjectModal.tsx
<Dropdown
  options={selectedTags.map(t => ({ value: String(t.id), label: t.name }))}
  === 0 ? 'Pick tags above first' : 'Select a tag to filter'}
  disabled={selectedTags.length === 0}
  // ← no onChange or value
/>

Either wire it up to filter reorderItems or remove it until the feature is ready. Fix this →

Type Safety

4. GlobalStateContext — libraries typed as unknown

libraries: unknown | null;  // consumers must cast every time

getLibrariesList() returns StrapiLibrariesResponse | null when called from this context. Narrowing the type here eliminates downstream casts. Fix this →

5. createTag.ts — missing return type

createTag has no explicit return type annotation, unlike every other API function in this module. This leaves its resolved shape as any. Fix this →

Architecture / Consistency

6. getTagsList — App Router SSR branch in a Pages Router project

getTagsList has a typeof window === 'undefined' guard that dynamically imports next/headers (an App Router API). This project uses Pages Router (useRouter from next/router). In Pages Router SSR (getServerSideProps), next/headers is not available and this branch would throw. The comment says "when called from a Server Component" — but Pages Router has no Server Components.

If this is intentional preparation for an App Router migration, add a clear comment. If not, remove the dead branch.

7. reorderObjects / reorderShelves — no error handling

All other API functions in this module wrap their calls in try/catch and return null on failure. reorderObjects, reorderShelves, createObject, and uploadFile don't, so callers must handle every error path themselves. AddObjectModal does wrap the call in try/catch, but the inconsistency is a footgun for future callers. Fix this →

8. AuthContext — manual cookie manipulation instead of js-cookie

js-cookie is already a declared dependency. handleProviderSignIn and handleLogout manipulate document.cookie directly — including constructing Secure and SameSite attributes by hand. This is fragile and inconsistent with the getCookie helper used in useEffect. Use Cookies.remove() from js-cookie instead.

Minor / Cosmetic

9. UserProfile.tsx — LibraryIcon has an unusual aspect ratio

The icon is rendered at width={20} height={11}, which is not square and doesn't match the other menu icons in the dropdown (Settings uses Image with no explicit non-square dimensions). Confirm this is intentional for the SVG's natural aspect ratio.

10. Incomplete hy locale in UserProfile

The hy (Armenian) locale label for myLibrary is 'My Library' (English), while ru already has the correct translation. This was likely an oversight. Fix this →

11. Three open TODOs blocking completeness

// TODO: hook up URL metadata auto-fetch (YouTube / Spotify / generic OG)
// TODO: drop the client-side `slug` once Strapi makes `object.slug` optional
// TODO: drop once Strapi sets `owner` in `beforeCreate`

All are in AddObjectModal.tsx. The slug and owner hacks are server-side dependencies — fine to ship with comments. The URL metadata fetch is a UI gap (the "Search" button does nothing); consider removing the button until it's wired up, so users don't click a no-op control.

@claude, review this again.

Claude finished @MaryWylde's task in 3m 26s —— View job

PR Review: Feat/library

• Gather PR context
• Review contexts (AuthContext, GlobalStateContext, DashboardContext)
• Review API layer
• Review component architecture (atoms, molecules, organisms)
• Review modified files (Header, Navbar, UserProfile)
• Check if prior review issues were addressed
• Post comprehensive review

Good progress since the last pass — most of the previous issues have been addressed. Notes below on what's still open, what's resolved, and three new bugs surfaced in this read.

Prior Issues: Resolved ✅

Still Open From Previous Review

Bug: AddObjectModal — "Select tag to reorder" dropdown is dead UI

The Dropdown at AddObjectModal.tsx:751–763 still has no onChange or value. It renders but does nothing. Remove it until the feature is wired up, or add the filter logic. Fix this →

Minor: UserProfile.tsx — LibraryIcon at non-square dimensions

LibraryIcon is rendered at width={20} height={11} (UserProfile.tsx:153). All other icons in the dropdown are square. Confirm this is intentional for the SVG's natural proportions, or correct to width={16} height={16}.

Minor: AddObjectModal — "Search" button is a no-op

The "Search" button in the sourceUrl field still calls an empty click handler (AddObjectModal.tsx:618–621). Users clicking it will get no feedback. Remove the button until the URL metadata fetch is implemented. Fix this →

New Issues

Bug 1: Sidebar.tsx — library dropdown navigates by numeric ID, not username

The dropdown options are built with lib.id (numeric Strapi ID) as the value:

// Sidebar.tsx:137–140
const dropdownOptions = libraryCards.map(lib => ({
  value: String(lib.id),  // ← numeric id
  label: lib.libraryName,
}));

handleLibraryChange then navigates to /library/${libraryId}, producing /library/123 instead of /library/johndoe. The URL route is [username], so this lands on a 404 or the wrong page. LibraryCard.tsx:27 correctly uses username ?? id — Sidebar should do the same.

Additionally, isViewingSelectedLibrary compares selectedLibrary.username === currentLibraryId (where currentLibraryId is the slug from the URL), but selectedLibraryId state holds a numeric ID — so this comparison is always false. Fix this →

Bug 2: Sidebar.tsx — tag slugs collide on duplicate names

Both handleCreateTag and handleEditTag generate slugs as ${formData.name.toLowerCase()}-1 (hardcoded -1 suffix). If the user creates two tags with different names that normalize to the same string (or renames a tag to match an existing one), both end up with identical slugs. Strapi enforces unique slugs, so the second write fails silently — the error is caught and re-thrown, but the UI shows no feedback from the catch in handleCreateTag. Use a timestamp suffix (like Date.now()) to guarantee uniqueness. Fix this →

Security: ObjectOverviewModal.tsx — dangerouslySetInnerHTML on user-supplied description

// ObjectOverviewModal.tsx:500–502
<div
  className={styles.description}
  dangerouslySetInnerHTML={{ __html: attributes.description }}
/>

description is user-provided content stored in Strapi. If Strapi does not sanitize HTML on write (CKEditor data is rich text and typically is not stripped), this is a stored XSS vector. Add client-side sanitization with DOMPurify.sanitize() before passing to __html. Fix this →

Architecture Note

LibraryToolbar — search input is wired but does nothing

LibraryToolbar.tsx:104–105 has a comment // Search is UI-only for now — no query wiring yet and the search state setSearch is never consumed downstream. The input renders and the user can type in it, but there's no filter applied. Similar to the "Search" button in AddObjectModal — either wire it or remove it. If left in, at minimum add a placeholder or tooltip indicating "Coming soon" so users know it's unimplemented.